Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Update templates to CR format #488

Merged
merged 7 commits into from
May 27, 2019
Merged

chore: Update templates to CR format #488

merged 7 commits into from
May 27, 2019

Conversation

WilcoFiers
Copy link
Member

Updating the template for ACT Rules to follow the new CR format. Following this PR, we'll start updating all of the published rules to use the new format as well.

Copy link
Member

@carlosapaduarte carlosapaduarte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just had one question regarding the (absence of) values for the forConformance attribute

pages/design/atomic-template-empty.md Outdated Show resolved Hide resolved
pages/design/composite-template-empty.md Outdated Show resolved Hide resolved
pages/design/atomic-template-empty.md Outdated Show resolved Hide resolved
## Test Procedure

### Applicability
## Applicability

The rule applies to any (??) element ...
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we make it "This rule applies to..." since this is the wording we mostly use.
And should it be a comment instead, since this is not always the wording used?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would still like to see this changed, if I am to approve this PR.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The is a definite article in English grammar used when the author/ reader knows exactly what is being referred to.

Eg:
It is the newest building in the town.
Who is the president of Madagascar?

Given we are in the context of the rule. I believe the is the right usage over this.

pages/design/atomic-template-empty.md Show resolved Hide resolved
pages/design/atomic-template-empty.md Show resolved Hide resolved
pages/design/atomic-template-empty.md Show resolved Hide resolved
pages/design/composite-template-empty.md Outdated Show resolved Hide resolved
pages/design/composite-template-empty.md Outdated Show resolved Hide resolved
pages/design/rule-template.md Show resolved Hide resolved
pages/design/rule-template.md Show resolved Hide resolved
pages/design/atomic-template-empty.md Show resolved Hide resolved
Copy link
Collaborator

@annethyme annethyme left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@WilcoFiers, could you have a look at the things market as "FOR TEMPLATE UPDATE OF RULES" asap?
We need a few answers before making trouble in too many rules...

pages/design/atomic-template-empty.md Outdated Show resolved Hide resolved
pages/design/atomic-template-empty.md Outdated Show resolved Hide resolved
pages/design/atomic-template-empty.md Outdated Show resolved Hide resolved
pages/design/atomic-template-empty.md Outdated Show resolved Hide resolved
passed: satisfied | further testing needed
inapplicable: satisfied | further testing needed

aria11:some-id: # <Some heading in WAI-ARIA>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For WAI-ARIA, can we use the id's that are actually used in WAI-ARIA 1.1, e.g. "#region" (https://www.w3.org/TR/wai-aria-1.1/#region) or "#requiredState" (https://www.w3.org/TR/wai-aria-1.1/#requiredState) ?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given we are using x.x.x everywhere, I think this should use the came convention here.

It may be worth adding an example for each of the sections, but again this could be explained in this document - https://github.com/act-rules/act-rules.github.io/pull/488/files#r284189403

pages/design/atomic-template-empty.md Outdated Show resolved Hide resolved
pages/design/atomic-template-empty.md Show resolved Hide resolved
## Test Procedure

### Applicability
## Applicability

The rule applies to any (??) element ...
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would still like to see this changed, if I am to approve this PR.

pages/design/atomic-template-empty.md Show resolved Hide resolved
@annethyme annethyme mentioned this pull request May 9, 2019
9 tasks
@jeeyyy jeeyyy assigned jeeyyy and unassigned WilcoFiers May 15, 2019
@jeeyyy jeeyyy requested a review from annethyme May 15, 2019 10:39
@jeeyyy jeeyyy dismissed annethyme’s stale review May 15, 2019 10:42

Updated. Please review again.

@jeeyyy jeeyyy requested a review from ShadowBB May 18, 2019 10:02
@WilcoFiers
Copy link
Member Author

@annethyme I'm merging this PR. If you have any further concerns, please open an issue.

@WilcoFiers WilcoFiers merged commit 7681f39 into develop May 27, 2019
@jeeyyy jeeyyy deleted the cr-template branch July 25, 2019 10:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants